@jcharkow I propagated changes to the class plotting methods, the tutorials and the manuscript figures. Let me know if I missed any other places

Sorry just looking at this again I'm not a huge fan of adding the trasitionGroup as an argument because could lead to breaking functionality.

E.g. what if a different transition group is provided than the one that is plotted?

I think a better implementation might be to create a dummy transitionGroup from the current plotting object and then do the check.

OR (possibly easier)

make add_peak_boundaries a private method that is invoked by the plot() function. If a transition list is supplied then the boundaries will be plotted.

Let me know your thoughts

@jcharkow, good point. I think we can just make add_peak_boundaries a private method. The InteractivePlotter class should handle whether to plot the boundaries or not when features are provided to plot.plot_chromatogram(transitionGroup, features).

I'm not sure why TransitionGroup::plot does this separately outside the interactive plotter?

Sorry just looking at this again I'm not a huge fan of adding the trasitionGroup as an argument because could lead to breaking functionality.
E.g. what if a different transition group is provided than the one that is plotted?
I think a better implementation might be to create a dummy transitionGroup from the current plotting object and then do the check.
OR (possibly easier)
make add_peak_boundaries a private method that is invoked by the plot() function. If a transition list is supplied then the boundaries will be plotted.
Let me know your thoughts

@jcharkow, good point. I think we can just make add_peak_boundaries a private method. The InteractivePlotter class should handle whether to plot the boundaries or not when features are provided to plot.plot_chromatogram(transitionGroup, features).

I'm not sure why TransitionGroup::plot does this separately outside the interactive plotter?

The idea behind the plot() function was to allow for a light wrapper for quick plotting with less customizability but without having to create a plotting object.

did you want to work on this or should I?

Sorry just looking at this again I'm not a huge fan of adding the trasitionGroup as an argument because could lead to breaking functionality.
E.g. what if a different transition group is provided than the one that is plotted?
I think a better implementation might be to create a dummy transitionGroup from the current plotting object and then do the check.
OR (possibly easier)
make add_peak_boundaries a private method that is invoked by the plot() function. If a transition list is supplied then the boundaries will be plotted.
Let me know your thoughts

@jcharkow, good point. I think we can just make add_peak_boundaries a private method. The InteractivePlotter class should handle whether to plot the boundaries or not when features are provided to plot.plot_chromatogram(transitionGroup, features).
I'm not sure why TransitionGroup::plot does this separately outside the interactive plotter?

The idea behind the plot() function was to allow for a light wrapper for quick plotting with less customizability but without having to create a plotting object.

This is fine and makes sense, but I don't see why it's necessary to call add_peak_boundaries outside of the plotter.plot call, instead of just directly passing the features object to the plotter.plot call to generate peak boundaries if features is not none.

did you want to work on this or should I?

I just made some changes based on making add_peak_boundaries private (__add_peak_boundaries) so that only the interactive plotter class directly deals with plotting the boundaries if a feature object is passed.

Feel free to change if there's a better option of dealing with this

Sorry misunderstanding

Sorry just looking at this again I'm not a huge fan of adding the trasitionGroup as an argument because could lead to breaking functionality.
E.g. what if a different transition group is provided than the one that is plotted?
I think a better implementation might be to create a dummy transitionGroup from the current plotting object and then do the check.
OR (possibly easier)
make add_peak_boundaries a private method that is invoked by the plot() function. If a transition list is supplied then the boundaries will be plotted.
Let me know your thoughts

@jcharkow, good point. I think we can just make add_peak_boundaries a private method. The InteractivePlotter class should handle whether to plot the boundaries or not when features are provided to plot.plot_chromatogram(transitionGroup, features).
I'm not sure why TransitionGroup::plot does this separately outside the interactive plotter?

The idea behind the plot() function was to allow for a light wrapper for quick plotting with less customizability but without having to create a plotting object.

This is fine and makes sense, but I don't see why it's necessary to call add_peak_boundaries outside of the plotter.plot call, instead of just directly passing the features object to the plotter.plot call to generate peak boundaries if features is not none.

Sorry misunderstanding yes it does not make sense for add_peak_boundaries outside of plot call. I must have missed something in the implementation

@jcharkow feel free to merge when ready